Skip to content

fix(rules): make ARCH-020 and ARCH-023 match ctx.ast() instead of raw text - #533

Merged
rhuanbarreto merged 4 commits into
mainfrom
claude/majestic-marinating-valley
Jul 26, 2026
Merged

fix(rules): make ARCH-020 and ARCH-023 match ctx.ast() instead of raw text#533
rhuanbarreto merged 4 commits into
mainfrom
claude/majestic-marinating-valley

Conversation

@rhuanbarreto

@rhuanbarreto rhuanbarreto commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Summary

  • ARCH-020/glob-scan-dot regexed raw source text for .scan( calls, so a comment or string literal merely mentioning .scan() was misreported as a violation. Fixes fix(rules): ARCH-020/glob-scan-dot matches .scan() inside comments and strings #513.
  • ARCH-023/scan-confined-to-fallback-modules had the exact same bug (its own comment said "Same call-site detection as ARCH-020's glob-scan-dot rule") — found while fire-testing the ARCH-020 fix, and fixed here too.
  • Both rules now parse via ctx.ast(file, "typescript") and use ctx.findAstNodes(tree, "CallExpression") to inspect real <expr>.scan(...) call sites structurally — comments and strings can no longer match by construction.
  • Since node.loc isn't trustworthy for "typescript" (ARCH-022), line numbers are re-located in the original source by blanking comments/strings first, so the re-location search itself can't land on a .scan( inside one either.
  • ARCH-022-ast-aware-rule-context.md gets a Consequences note documenting ctx.findAstNodes() as the preferred replacement for a hand-rolled walk() helper — none of ARCH-022's own example rules use it yet, since it shipped after they were written (feat(engine): ctx.findAstNodes() generic AST node collector #486).

Test plan

  • ARCH-020: a genuine glob.scan({ cwd }) without dot: true is still flagged, at the correct line; a comment/string mentioning .scan() with no real call produces zero false positives.
  • ARCH-023: a genuine .scan() call outside the two allowlisted modules is still flagged, at the correct line; a comment/string mentioning .scan() with no real call produces zero false positives.
  • bun run validate passes (lint, typecheck, format, 1812 tests, archgate check 49/49, knip, build).
  • archgate:reviewer skill: APPROVED across General/Process, CI, and Distribution domains — 0 violations, 0 warnings.

glob-scan-dot regexed raw source for `.scan(` calls, so a comment or
string literal merely mentioning `.scan()` was misreported as a
violation (fixes #513). It now walks the ESTree via ctx.ast() /
ctx.findAstNodes() for real `<expr>.scan(...)` CallExpression nodes,
so comments and strings can't match by construction. Line numbers are
re-located in the original source (ctx.ast()'s loc isn't trustworthy
for "typescript", per ARCH-022), with comments/strings blanked first
so the re-location search has the same guarantee.

Also documents ctx.findAstNodes() as the preferred replacement for a
hand-rolled walk() helper in ARCH-022, since none of its own example
rules used it yet (it shipped after they were written).

Fire-tested both directions: a genuine scan() without dot:true still
fails, and a comment/string containing the literal text no longer does.

Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

ARCH-020 now parses TypeScript files and inspects AST CallExpression nodes for .scan(...) usage, checking for a dot property while excluding comments and strings. It maps violations to source lines using newline-preserving source processing. Related ADRs document the implementation and AST rule-context capability, while internal notes record the corresponding unresolved ARCH-023 raw-text matching issue.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes satisfy #513 by using ctx.ast() and AST node inspection while preserving the expected failure cases.
Out of Scope Changes check ✅ Passed No clearly unrelated changes stand out; the docs and memory updates stay aligned with the same rule-fix work.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check ✅ Passed The title clearly summarizes the main change: switching ARCH-020 and ARCH-023 from raw-text matching to ctx.ast()-based detection.
Description check ✅ Passed The description is directly related to the changes and test plan, accurately describing the AST-based fixes and documentation update.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 26, 2026

Copy link
Copy Markdown

Deploying archgate-cli with  Cloudflare Pages  Cloudflare Pages

Latest commit: db8bab2
Status: ✅  Deploy successful!
Preview URL: https://be03a370.archgate-cli.pages.dev
Branch Preview URL: https://claude-majestic-marinating-v.archgate-cli.pages.dev

View logs

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.archgate/adrs/ARCH-020-glob-scan-include-dotfiles.md:
- Line 58: Update the ARCH-020 rule description to state that an argument must
contain an object literal with the dot key set to true, matching the Decision
requirement and the corrected hasDotOption behavior. Keep the structural
AST-based detection description unchanged.
- Line 58: Align ARCH-020’s documented discovery scope with the implementation:
verify whether ctx.scopedFiles is already restricted to src/**; if not, update
the rule’s file filter to require both a src/ path and .ts extension. Then
revise the ADR text to describe the resulting scope accurately, keeping the
implementation and documented contract consistent.

In @.archgate/adrs/ARCH-020-glob-scan-include-dotfiles.rules.ts:
- Around line 20-36: Update hasDotOption to require the matched dot property’s
value to be the boolean literal true, rather than treating key presence alone as
sufficient. Continue supporting both identifier and string-literal dot keys,
while rejecting dot: false and other non-true values.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 13f66347-274f-416b-94f8-489ee273aaa1

📥 Commits

Reviewing files that changed from the base of the PR and between 2b20d69 and c8a7279.

📒 Files selected for processing (5)
  • .archgate/adrs/ARCH-020-glob-scan-include-dotfiles.md
  • .archgate/adrs/ARCH-020-glob-scan-include-dotfiles.rules.ts
  • .archgate/adrs/ARCH-022-ast-aware-rule-context.md
  • .claude/agent-memory/archgate-developer/MEMORY.md
  • .claude/agent-memory/archgate-developer/project_rules_engine_internals.md
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
  • GitHub Check: Smoke Test (Windows) / Windows
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Analyze (csharp)
  • GitHub Check: Analyze (java-kotlin)
  • GitHub Check: Cloudflare Pages
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2026-06-11T12:50:28.661Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 406
File: .claude/agent-memory/archgate-developer/feedback_prefer_tests_over_adr_rules.md:8-18
Timestamp: 2026-06-11T12:50:28.661Z
Learning: In `archgate/cli`, for markdown files under `.claude/agent-memory/`, follow the established convention: use YAML frontmatter (with a `name:` field used as the document title) and do not require a top-level `#` (H1) heading. During code review, do not flag missing first-line/first-top-level H1 headings (e.g., MD041) for these agent-memory files since markdownlint is not part of the repo’s `bun run validate` lint pipeline (oxlint/oxfmt only).

Applied to files:

  • .claude/agent-memory/archgate-developer/MEMORY.md
  • .claude/agent-memory/archgate-developer/project_rules_engine_internals.md
📚 Learning: 2026-07-25T00:05:20.592Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 496
File: .claude/agent-memory/archgate-developer/project_test_isolation_gotchas.md:10-10
Timestamp: 2026-07-25T00:05:20.592Z
Learning: When reviewing documentation/agent-memory entries under `.claude/agent-memory/**`, do not enforce GEN-004’s “forward-only” comment/narrative requirement. These entries are allowed to keep historical/past-tense incident narratives and dated markers (e.g., `Found YYYY-MM-DD`) because the context is intended to help future agents evaluate edge cases. Outside this scope, GEN-004’s forward-only rule should still apply.

Applied to files:

  • .claude/agent-memory/archgate-developer/MEMORY.md
  • .claude/agent-memory/archgate-developer/project_rules_engine_internals.md
📚 Learning: 2026-07-11T13:03:15.386Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 467
File: .archgate/adrs/ARCH-011-consistent-project-root-resolution.md:0-0
Timestamp: 2026-07-11T13:03:15.386Z
Learning: For Markdown files formatted by oxfmt (especially ADRs), avoid inline code spans that contain escaped backticks, e.g. `\`...\`` inside a single `` `...` `` span. oxfmt may mis-parse these and, on re-format, can collapse spaces after later inline code spans on the same line, effectively removing any manually re-added spacing. Instead, rephrase the text so the message stays plain quoted text, and put any embedded command/fragment that needs code formatting (e.g., `archgate init`) in its own separate inline code span; keep surrounding punctuation/spacing outside the code span.

Applied to files:

  • .archgate/adrs/ARCH-022-ast-aware-rule-context.md
  • .archgate/adrs/ARCH-020-glob-scan-include-dotfiles.md
📚 Learning: 2026-07-25T16:24:51.133Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 501
File: .archgate/adrs/ARCH-003-output-formatting.md:0-0
Timestamp: 2026-07-25T16:24:51.133Z
Learning: In Archgate ADRs (.archgate/adrs/*.md), omit quantitative claims (e.g., token savings, benchmarks, performance deltas) unless they are backed by a reproducible measurement and supported by a single cited reference. If you cannot satisfy both (reproducible measurement + exactly one cited reference), describe the benefit qualitatively and tie it to the relevant policy/requirements instead of using numeric estimates.

Applied to files:

  • .archgate/adrs/ARCH-022-ast-aware-rule-context.md
  • .archgate/adrs/ARCH-020-glob-scan-include-dotfiles.md
📚 Learning: 2026-07-25T22:03:17.073Z
Learnt from: rhuanbarreto
Repo: archgate/cli PR: 501
File: .archgate/adrs/ARCH-015-cli-command-documentation-coverage.md:17-18
Timestamp: 2026-07-25T22:03:17.073Z
Learning: When updating an ADR that documents rule discovery/enforcement behavior, ensure the ADR’s stated discovery contract matches the implementation in code. If the rule only discovers commands by scanning `src/commands/*.ts` and `src/commands/*/index.ts`, the ADR must not claim it also inspects command registration calls elsewhere (e.g., `src/cli.ts`). Any ADR language that changes the documented contract should be treated as a normative change to behavior and aligned with the corresponding implementation/issue, not as prose-only documentation compression.

Applied to files:

  • .archgate/adrs/ARCH-022-ast-aware-rule-context.md
  • .archgate/adrs/ARCH-020-glob-scan-include-dotfiles.md
🪛 markdownlint-cli2 (0.23.0)
.claude/agent-memory/archgate-developer/project_rules_engine_internals.md

[warning] 8-8: First line in a file should be a top-level heading

(MD041, first-line-heading, first-line-h1)

🔇 Additional comments (7)
.claude/agent-memory/archgate-developer/MEMORY.md (1)

42-42: LGTM!

.claude/agent-memory/archgate-developer/project_rules_engine_internals.md (1)

9-9: LGTM!

.archgate/adrs/ARCH-020-glob-scan-include-dotfiles.rules.ts (3)

9-18: LGTM!

Also applies to: 38-105, 117-127, 129-138, 148-161


113-114: 🎯 Functional Correctness

No change needed — ctx.scopedFiles is already scoped to the ADR files patterns before the rule runs.

ARCH-020’s frontmatter declares files: ["src/**/*.ts"], and the runner passes that to resolveScopedFiles() before creating ctx, so the rule cannot scan outside src/**/*.ts.


140-146: 🩺 Stability & Availability

No double-read desync issue to address.

ctx.readFile() shares the same cachedFileText cache that ctx.ast() uses for TypeScript/JavaScript source loading, so this concern does not apply.

.archgate/adrs/ARCH-020-glob-scan-include-dotfiles.md (1)

67-70: LGTM!

.archgate/adrs/ARCH-022-ast-aware-rule-context.md (1)

115-115: LGTM!

Comment thread .archgate/adrs/ARCH-020-glob-scan-include-dotfiles.md Outdated
Comment thread .archgate/adrs/ARCH-020-glob-scan-include-dotfiles.rules.ts Outdated
Same bug class as ARCH-020 (#513), confirmed live in this rule too: it
regexed raw source for `.scan(` call sites in src/engine/, so a comment
or string literal merely mentioning `.scan()` in a non-allowlisted file
was misreported as a violation. Ports ARCH-020's fix — walk the ESTree
via ctx.ast()/ctx.findAstNodes() for real `<expr>.scan(...)` call sites,
with the same comment/string-blanking approach for line re-location.

Fire-tested both directions: a genuine scan() call outside the two
allowlisted modules is still flagged, and a comment/string containing
the literal text no longer is.

Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
@rhuanbarreto rhuanbarreto changed the title fix(rules): ARCH-020/glob-scan-dot matches ctx.ast() instead of raw text fix(rules): ARCH-020 and ARCH-023 match ctx.ast() instead of raw text Jul 26, 2026
CodeRabbit review on PR #533: hasDotOption checked only for the dot
key's presence, not its value, so `glob.scan({ cwd, dot: false })`
passed the check while reproducing the exact silent-skip bug ARCH-020
exists to prevent. A literal value must now be true; a non-literal
value (identifier, expression) can't be resolved statically and stays
treated as compliant, avoiding false positives on constants/config.

Also updates the ADR's Compliance wording to match, and confirms (by
reading resolveScopedFiles/git-files.ts) that a second review finding
-- the rule's file scope allegedly not being confined to src/** -- is
already false: ctx.scopedFiles is pre-scoped to the ADR's files glob
before the rule runs, matching CodeRabbit's own follow-up on the same
finding.

Fire-tested: dot: false is now flagged; dot: true and a non-literal
dot: someConst are not.
Signed-off-by: Rhuan Barreto <rhuan@barreto.work>
@rhuanbarreto rhuanbarreto changed the title fix(rules): ARCH-020 and ARCH-023 match ctx.ast() instead of raw text fix(rules): make ARCH-020 and ARCH-023 match ctx.ast() instead of raw text Jul 26, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Code Coverage

Metric Value
Lines 91.3% (8354 / 9155)
Threshold 90% minimum — met
Platforms Linux + Windows

Full HTML report available in workflow artifacts.

Per-directory breakdown
Directory Coverage Lines
src/commands/ 88.7% 1966 / 2216
src/engine/ 93.8% 2230 / 2378
src/formats/ 98.7% 148 / 150
src/helpers/ 90.9% 4010 / 4411

@rhuanbarreto
rhuanbarreto merged commit ad5529b into main Jul 26, 2026
28 of 32 checks passed
@rhuanbarreto
rhuanbarreto deleted the claude/majestic-marinating-valley branch July 26, 2026 13:23
@archgatebot archgatebot Bot mentioned this pull request Jul 26, 2026
rhuanbarreto pushed a commit that referenced this pull request Jul 26, 2026
# archgate

## [0.51.0](v0.50.0...v0.51.0)
(2026-07-26)

### Features

* **adrs:** enforce concise, forward-only code comments (GEN-004)
([#496](#496))
([9a114b3](9a114b3)),
references [#2123](https://github.com/archgate/cli/issues/2123)
* **adrs:** flag stray files at the repository root (GEN-005)
([#535](#535))
([6a6e765](6a6e765)),
closes [#514](#514), references
[#500](#500)
* **engine:** add ctx.readYAML and ctx.checkCase rule helpers
([#497](#497))
([c5d82c5](c5d82c5)),
closes [#490](#490), references
[#490](#490)
[#491](#491)
[#499](#499)
[#499](#499)
[#499](#499)
[#499](#499)
* report truncated ADR briefings, trim the ADR corpus 15.6%, add GEN-005
briefing budget ([#501](#501))
([a9dab40](a9dab40))

### Bug Fixes

* **docs:** relocate ADR content to clear briefing-budget warnings
([#531](#531))
([c7419b3](c7419b3))
* **docs:** restore pt-br diacritics and enforce locale content
integrity ([#523](#523))
([db39104](db39104)),
closes [#516](#516), references
[#231](#231)
* **engine:** allow symlinks that resolve inside the project root
([#500](#500))
([387bf15](387bf15))
* **engine:** reject rule-file reads through a symlinked ancestor
directory ([#499](#499))
([a555f9d](a555f9d)),
references [#497](#497)
[#491](#491)
[#497](#497)
[#497](#497)
[#497](#497)
[#497](#497)
* **engine:** scan top-level export declarations with a null source
([#493](#493))
([d07db03](d07db03)),
closes [#491](#491)
* **engine:** stop dropping AST nodes with exotic literal values
([#494](#494))
([0015542](0015542)),
closes [#493](#493)
[#493](#493)
* **lint:** resolve no-bare-env-restore by captured key and lexical
scope ([#524](#524))
([7094a3a](7094a3a)),
closes [#498](#498)
* **rules:** make ARCH-020 and ARCH-023 match ctx.ast() instead of raw
text ([#533](#533))
([ad5529b](ad5529b)),
closes [#513](#513), references
[#486](#486)
* **tests:** replace bun:test anti-patterns with idiomatic patterns
([#512](#512))
([bcb086f](bcb086f))

---
This PR was generated with
[simple-release](https://github.com/TrigenSoftware/simple-release).

<details>
<summary>📄 Cheatsheet</summary>
<br>



You can configure the bot's behavior through a pull request comment
using the `!simple-release/set-options` command.

### Command Format

````md
!simple-release/set-options

```json
{
  "bump": {},
  "publish": {}
}
```
````

### Useful Parameters

#### Bump

| Parameter | Type | Description |
|-----------|------|-------------|
| `version` | `string` | Force set specific version |
| `as` | `'major' \| 'minor' \| 'patch' \| 'prerelease'` | Release type
|
| `prerelease` | `string` | Pre-release identifier (e.g., "alpha",
"beta") |
| `firstRelease` | `boolean` | Whether this is the first release |
| `skip` | `boolean` | Skip version bump |
| `byProject` | `Record<string, object>` | Per-project bump options for
monorepos |

#### Publish

| Parameter | Type | Description |
|-----------|------|-------------|
| `skip` | `boolean` | Skip publishing |
| `access` | `'public' \| 'restricted'` | Package access level |
| `tag` | `string` | Tag for npm publication |

### Usage Examples

#### Force specific version

````md
!simple-release/set-options

```json
{
  "bump": {
    "version": "2.0.0"
  }
}
```
````

#### Force major bump

````md
!simple-release/set-options

```json
{
  "bump": {
    "as": "major"
  }
}
```
````

#### Create alpha pre-release

````md
!simple-release/set-options

```json
{
  "bump": {
    "prerelease": "alpha"
  }
}
```
````

#### Publish with specific access and tag

````md
!simple-release/set-options

```json
{
  "bump": {
    "prerelease": "beta"
  },
  "publish": {
    "access": "public",
    "tag": "beta"
  }
}
```
````

### Custom Changelog Preamble

You can add custom markdown to the top of the changelog (right after the
version header) using the `!simple-release/set-preamble` command. The
markdown after the command line becomes the preamble.

```md
!simple-release/set-preamble

## What's new?

- The website was completely redesigned
- The new API gives you awesome possibilities
```

In a monorepo, pass the full package name after the command to target a
single package's changelog. Wrap the name in backticks so GitHub keeps
it as text instead of a mention:

```md
!simple-release/set-preamble `@your-org/core`

## Core changes

- New plugin system
```

Use one comment per package, plus one without a name for the whole
release.

### Access Restrictions

The commands can only be used by users with permissions:
- repository owner
- organization member
- collaborator

### Notes

- The last comment with `!simple-release/set-options` command takes
priority
- The last `!simple-release/set-preamble` comment per package takes
priority
- JSON must be valid, otherwise the `set-options` command will be
ignored
- Parameters apply only to the current release execution
- The commands can be updated by editing the comment or adding a new one


</details>

<!--
  Please do not edit this comment.
  simple-release-pull-request: true
  simple-release-branch-from: release
  simple-release-branch-to: main
-->

Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(rules): ARCH-020/glob-scan-dot matches .scan() inside comments and strings

1 participant